Skip to content

Conversation

@ashishmohapatra240
Copy link
Contributor

@ashishmohapatra240 ashishmohapatra240 commented Oct 24, 2025

Fixes #3288

Prevents infinite recursion in fit_to_bezpath by:

  • Filtering out degenerate cubic beziers
  • Capping max segments at 10k to prevent capacity overflow
Graphite.mp4

@TrueDoctor
Copy link
Member

I don't love lowering of the accuracy, is that a necessary change?

@ashishmohapatra240
Copy link
Contributor Author

i initially relaxed the accuracy from 1e-3 to 1e-2 thinking it would reduce recursion depth in fit_to_bezpath, reverted to 1e-3.

@ashishmohapatra240
Copy link
Contributor Author

Hey, @TrueDoctor Let me know if this works!

@ashishmohapatra240
Copy link
Contributor Author

simplified with then_some()!, much cleaner!

Comment on lines 27 to 28
let is_degenerate =
(cubic_bez.p1 - start).hypot() < MAX_ABSOLUTE_DIFFERENCE && (cubic_bez.p2 - start).hypot() < MAX_ABSOLUTE_DIFFERENCE && (cubic_bez.p3 - start).hypot() < MAX_ABSOLUTE_DIFFERENCE;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add a quick comment about when this is true and why we return None in those cases? Also could you use start.distance(cubic_bez.p1) instead to make this more readable? If we introduced a squared constant, we could also use start.distance_squared(cubic_bez.p1) which would avoid the cost of computing the square root

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added comment explaining degenerate curves and switched to distance_squared(), please check.

Copy link
Member

@TrueDoctor TrueDoctor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alright, looking good!

@TrueDoctor TrueDoctor enabled auto-merge (squash) October 27, 2025 11:58
@TrueDoctor TrueDoctor merged commit 6ca25e4 into GraphiteEditor:master Oct 27, 2025
3 of 4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Kurbo capacity overflow with Offset Path node

2 participants